router-tests/telemetry/telemetry_test.go (6)

9093-9094: Same as above for AccessController args and helper extraction.

---

9140-9141: Same as above for AccessController args and helper extraction.

---

9224-9225: Same as above for AccessController args and helper extraction.

---

9291-9292: Same as above for AccessController args and helper extraction.

---

9347-9348: Same as above for AccessController args and helper extraction.

---

9554-9555: Same as above for AccessController args and helper extraction.

router/pkg/config/fixtures/full.yaml (1)

23-26: Clarify precedence with deprecated introspection_enabled

This fixture defines both the deprecated introspection_enabled and the new introspection.enabled. Please document or enforce precedence; ideally, drop the deprecated field from example fixtures to avoid confusion.

Optional cleanup (outside the changed hunk):

-introspection_enabled: true

router/core/router_config.go (1)

53-53: Expose new introspection settings in anonymized Usage() (without leaking the token)

Add two booleans to Usage: whether skip-auth is enabled and whether a token is configured (do not export the token value).

Apply this diff:

@@ func (c *Config) Usage() map[string]any {
-	usage["introspection"] = c.introspection
+	usage["introspection"] = c.introspection
+	usage["introspection_skip_authentication"] = c.introspectionConfig.SkipAuthentication
+	usage["introspection_token_set"] = c.introspectionConfig.Token != ""

router-tests/testenv/testenv.go (1)

1423-1425: Avoid mixing deprecated and new introspection APIs

Using both WithIntrospection(true) and WithIntrospectionConfig(...) invites drift. Prefer the new WithIntrospectionConfig in tests.

Apply this diff:

-		core.WithIntrospection(true),
-		core.WithIntrospectionConfig(config.IntrospectionConfiguration{
-			Enabled: true,
-		}),
+		core.WithIntrospectionConfig(config.IntrospectionConfiguration{
+			Enabled: true,
+		}),

router/pkg/config/testdata/config_full.json (1)

462-466: Document header semantics for the token

Please confirm in docs/schema whether the token must be sent as Authorization: Bearer <token> or as a raw value, and how it interacts with SkipAuthentication. Define precedence if both are set.

I can open a docs follow-up PR once confirmed.

router-tests/prometheus_test.go (1)

4131-4132: Reduce repetition of positional bool args with a test helper

To avoid future churn and improve readability across tests, consider a helper returning core.Option.

Apply this local replacement after adding the helper:

- core.WithAccessController(core.NewAccessController(authenticators, true, false, "")),
+ testenv.WithDefaultAccessController(authenticators, true),

Add to testenv (or a shared test helper):

package testenv

import (
  "github.com/wundergraph/cosmo/router/core"
  "github.com/wundergraph/cosmo/router/pkg/authentication"
)

func WithDefaultAccessController(authenticators []authentication.Authenticator, requireAuth bool) core.Option {
  return core.WithAccessController(core.NewAccessController(authenticators, requireAuth, false, ""))
}

router/core/router.go (2)

225-233: Make deprecation sync deterministic and avoid noisy warnings

Current logic logs a deprecation warning whenever the old flag is false, even if the new config wasn’t used and there’s no divergence. Also, the else-if branch silently rewrites the old flag without warning. Prefer: warn only when values diverge and make “false wins” explicit for safety.

-	// handle introspection config deprecation
-	// if either the old deprecated or the new config is set to false, introspection is disabled
-	if !r.introspection {
-		r.introspectionConfig.Enabled = r.introspection
-		r.logger.Warn("The introspection_enabled option is deprecated. Use the introspection.enabled option in the config instead.")
-	} else if !r.introspectionConfig.Enabled {
-		r.introspection = r.introspectionConfig.Enabled
-	}
+	// handle introspection config deprecation
+	// If values diverge, prefer safer behavior: false wins, and emit a single deprecation warning.
+	if r.introspection != r.introspectionConfig.Enabled {
+		if !r.introspection || !r.introspectionConfig.Enabled {
+			r.introspection = false
+			r.introspectionConfig.Enabled = false
+		}
+		r.logger.Warn("The introspection_enabled option is deprecated. Use introspection.enabled instead. Diverging values detected; 'false' takes precedence.")
+	}

---

1560-1565: Avoid shadowing the imported config package

Rename the parameter to align with nearby options and avoid overshadowing the config import.

-func WithIntrospectionConfig(config config.IntrospectionConfiguration) Option {
+func WithIntrospectionConfig(cfg config.IntrospectionConfiguration) Option {
 	return func(r *Router) {
-		r.introspectionConfig = config
+		r.introspectionConfig = cfg
 	}
 }

router-tests/websocket_test.go (1)

140-140: Signature updates look correct; consider a small helper and verify for stragglers

All call sites correctly pass the two new args (skip introspection auth, token). To reduce repetition/noise across tests, consider a tiny helper (e.g., newAccessController(authenticators, requireAuth bool)) that supplies the defaults false,"".

Run to ensure no remaining 2-arg constructor calls:

#!/bin/bash
# Find legacy usages of NewAccessController with exactly two arguments
rg -nP --type go '\bNewAccessController\s*\(\s*[^,()]+\s*,\s*[^,()]+\s*\)'

Also applies to: 189-189, 240-240, 299-299, 361-361, 421-421, 470-470, 886-886

router-tests/block_operations_test.go (1)

132-133: Updated constructor args are consistent with the new API

Looks good. Same optional helper suggestion as in other tests to avoid repeating false,"".

Also applies to: 286-287, 383-383

router-tests/telemetry/telemetry_test.go (1)

9044-9045: Confirm empty introspection token semantics; consider reducing repetition.

• You pass NewAccessController(authenticators, false, false, ""). Please verify that an empty token ("") is treated as “no token configured” and cannot accidentally match an empty Authorization header value. Also confirm that skip-introspection-auth=false keeps auth enforced for introspection.

• Minor: this 4-arg constructor appears multiple times. Consider a tiny helper to avoid call-site churn if the signature changes again.

Apply at call sites:

- core.WithAccessController(core.NewAccessController(authenticators, false, false, ""))
+ core.WithAccessController(newTestAccessController(authenticators))

Add once in this file (or testenv):

func newTestAccessController(authenticators core.Authenticators) core.AccessController {
	return core.NewAccessController(authenticators, false, false, "")
}

router/pkg/config/config.go (1)

986-991: Don’t serialize empty token values

Add omitempty so empty tokens aren’t emitted back to YAML or logs via config dumps.

 type IntrospectionConfiguration struct {
 	Enabled            bool   `yaml:"enabled" envDefault:"true" env:"INTROSPECTION_ENABLED"`
 	SkipAuthentication bool   `yaml:"skip_authentication" envDefault:"false" env:"INTROSPECTION_SKIP_AUTHENTICATION"`
-	Token              string `yaml:"token" env:"INTROSPECTION_TOKEN"`
+	Token              string `yaml:"token,omitempty" env:"INTROSPECTION_TOKEN"`
 }

router/core/supervisor_instance.go (2)

60-65: Enforce introspection token even without JWT authenticators

AccessController is only created when authenticators exist. If users configure only an introspection token (no JWT), it never gets enforced.

-	if len(authenticators) > 0 {
+	if len(authenticators) > 0 || cfg.IntrospectionConfig.SkipAuthentication || cfg.IntrospectionConfig.Token != "" {
 		options = append(options, WithAccessController(NewAccessController(
 			authenticators,
 			cfg.Authorization.RequireAuthentication,
 			cfg.IntrospectionConfig.SkipAuthentication,
 			cfg.IntrospectionConfig.Token,
 		)))
 	}

---

163-165: Use the new nested flag to control introspection

To avoid contradictory settings between deprecated and new flags, drive WithIntrospection from IntrospectionConfig.Enabled.

-		WithIntrospection(config.IntrospectionEnabled),
+		WithIntrospection(config.IntrospectionConfig.Enabled),
 		WithIntrospectionConfig(config.IntrospectionConfig),

router/core/access_controller.go (2)

20-24: Constructor grows boolean blindness; consider options struct.

Two trailing booleans make call‑sites error‑prone and reduce readability. Prefer a single options struct (or functional options) to carry authenticationRequired, skipIntrospectionAuth, and the token.

Also applies to: 26-33

---

100-112: Accept “Bearer ” form for the introspection token.

Current check requires the Authorization header to equal the raw token, which is surprising given normal usage. Allow both raw and “Bearer ” while keeping constant‑time compares.

Apply this diff:

 func (a *AccessController) isValidIntrospectionToken(r *http.Request) bool {
   // If no token is configured, allow introspection without authentication
   if len(a.introspectionAuthSkipToken) == 0 {
     return true
   }

   authHeader := r.Header.Get("Authorization")
   if authHeader == "" {
     return false
   }

-  return subtle.ConstantTimeCompare([]byte(authHeader), []byte(a.introspectionAuthSkipToken)) == 1
+  token := a.introspectionAuthSkipToken
+  // Accept exact token
+  if len(authHeader) == len(token) &&
+    subtle.ConstantTimeCompare([]byte(authHeader), []byte(token)) == 1 {
+    return true
+  }
+  // Accept "Bearer <token>"
+  bearer := "Bearer " + token
+  if len(authHeader) == len(bearer) &&
+    subtle.ConstantTimeCompare([]byte(authHeader), []byte(bearer)) == 1 {
+    return true
+  }
+  return false
 }

router-tests/authentication_test.go (3)

2509-2725: Great coverage; add a few hardening cases.

• Mixed query (introspection + normal field) must not bypass auth.
• GET /graphql with introspection in query param (if supported) should get the same treatment.
• Authorization: "Bearer " should also bypass when a token is configured (post‑change in AccessController).

I can append subtests for these three scenarios; want a patch?

---

2621-2631: Secret scanner false positive; replace high‑entropy test token.

Gitleaks flags the literal token as a generic API key. Use a clearly fake, low‑entropy value and centralize it.

Apply this diff (and reuse in all places):

@@
-        token := "0aG3TVhkZ2lPy8ULUPQLXZ4JFPfpxk"
+        // Non‑secret, low‑entropy value to avoid triggering secret scanners.
+        token := "introspection-token-for-tests" // gitleaks:allow
@@
-            header := http.Header{
-                "Authorization": []string{token},
-            }
+            header := http.Header{
+                "Authorization": []string{token},
+            }

Optionally, lift the token to a file‑level const near the other test constants and reference it here and at Line 2649.

---

2550-2555: Assert JSON semantically to reduce brittle string compares.

For these response bodies, require.JSONEq improves robustness and readability.

-            require.Equal(t, simpleIntrospectionExpectedData, string(data))
+            require.JSONEq(t, simpleIntrospectionExpectedData, string(data))

Also applies to: 2580-2584, 2606-2611, 2634-2639

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

router/core/router.go (1)

225-232: Strengthen deprecation handling and messaging when flags conflict

Currently you warn only when the legacy flag disables introspection; also warn when the new config disables it while the legacy flag is true, and standardize the path format to “/introspection/enabled”.

 // handle introspection config deprecation
 // if either the old deprecated or the new config is set to false, introspection is disabled
 if !r.introspection {
-   r.introspectionConfig.Enabled = r.introspection
-   r.logger.Warn("The introspection_enabled option is deprecated. Use the introspection.enabled option in the config instead.")
+   r.introspectionConfig.Enabled = r.introspection
+   r.logger.Warn("The legacy 'introspection_enabled' option is deprecated. Use '/introspection/enabled' instead.")
 } else if !r.introspectionConfig.Enabled {
-   r.introspection = r.introspectionConfig.Enabled
+   r.introspection = r.introspectionConfig.Enabled
+   r.logger.Warn("The legacy 'introspection_enabled' option is deprecated. '/introspection/enabled=false' overrides the legacy flag when they conflict.")
 }

router-tests/websocket_test.go (13)

195-195: Same WithAccessController wiring — OK

Matches the new API usage pattern.

---

241-242: OK to construct controller here as well

Pattern mirrors earlier block.

---

248-248: OK: controller passed via option

Consistent with prior change.

---

302-303: Controller ctor + error check — OK

No concerns.

---

309-309: WithAccessController usage — OK

---

362-363: Controller ctor with required=true — OK

Appropriate for “auth via initial payload” scenarios.

---

373-373: WithAccessController usage — OK

---

426-427: Controller ctor — OK

---

435-435: WithAccessController usage — OK

---

477-478: Controller ctor — OK

---

486-486: WithAccessController usage — OK

---

880-881: Controller ctor — OK

---

903-903: WithAccessController usage — OK

router/pkg/config/config.go (1)

1018-1020: Resolved env var collision on deprecated flag.
Removing env tag from IntrospectionEnabled prevents conflicts; nested config is now SoT.

router/core/graphql_prehandler.go (1)

358-389: Nice: no span leak—Authenticate span created only when used.
This addresses the earlier leak concern cleanly.

router/core/access_controller.go (1)

101-131: Support GET-based introspection detection.
Without this, ?query=… introspection over GET never bypasses auth.

-// isIntrospectionQuery checks if the operation in body is an introspection query.
-// It returns false if the operation is not an introspection query or we cannot parse it.
-func isIntrospectionQuery(operationProcessor *OperationProcessor, body []byte) bool {
-  if operationProcessor == nil || body == nil {
-    return false
-  }
-  operationKit, err := operationProcessor.NewKit()
-  if err != nil {
-    return false
-  }
-  defer operationKit.Free()
-  err = operationKit.UnmarshalOperationFromBody(body)
-  if err != nil {
-    return false
-  }
+// isIntrospectionRequest checks if the HTTP request is an introspection query (POST or GET).
+// It returns false if the operation is not introspection or cannot be parsed.
+func isIntrospectionRequest(r *http.Request, operationProcessor *OperationProcessor, body []byte) bool {
+  if operationProcessor == nil {
+    return false
+  }
+  operationKit, err := operationProcessor.NewKit()
+  if err != nil {
+    return false
+  }
+  defer operationKit.Free()
+  switch r.Method {
+  case http.MethodGet:
+    if err = operationKit.UnmarshalOperationFromURL(r.URL); err != nil {
+      return false
+    }
+  default:
+    if body == nil {
+      return false
+    }
+    if err = operationKit.UnmarshalOperationFromBody(body); err != nil {
+      return false
+    }
+  }
   err = operationKit.Parse()
   if err != nil {
     return false
   }
   isIntrospection, err := operationKit.isIntrospectionQuery()
   if err != nil {
     return false
   }
   return isIntrospection
 }

router-tests/testenv/testenv.go (1)

1422-1424: API update LGTM (consider explicit auth mode for clarity)

Passing Enabled: true is fine; consider also setting AuthenticationMode: "full" to make the test resilient to future default changes.

- core.WithIntrospection(true, config.IntrospectionConfiguration{
-   Enabled: true,
- }),
+ core.WithIntrospection(true, config.IntrospectionConfiguration{
+   Enabled:            true,
+   AuthenticationMode: "full",
+ }),

router/core/router.go (1)

1554-1558: Avoid shadowing the imported config package

The parameter name “config” shadows the imported package; rename to cfg for clarity.

-func WithIntrospection(enable bool, config config.IntrospectionConfiguration) Option {
+func WithIntrospection(enable bool, cfg config.IntrospectionConfiguration) Option {
 	return func(r *Router) {
 		r.introspection = enable
-		r.introspectionConfig = config
+		r.introspectionConfig = cfg
 	}
 }

router-tests/prometheus_test.go (1)

4122-4124: Optional: DRY up AccessController setup across tests

Consider a tiny helper to reduce repetition.

// testutil/test_access.go
func NewTestAccessController(t *testing.T, auth []authentication.Authenticator, required bool) *core.AccessController {
	t.Helper()
	ac, err := core.NewAccessController(auth, required, core.IntrospectionAuthModeFull, "")
	require.NoError(t, err)
	return ac
}

Then replace local constructions with testutil.NewTestAccessController(t, authenticators, true).

Also applies to: 4134-4134

router-tests/header_set_test.go (1)

278-280: Optional: extract a shared helper for AccessController creation

Same helper as suggested in prometheus tests would keep these call sites concise.

Also applies to: 287-287

router-tests/ratelimit_test.go (1)

273-275: Optional: unify controller setup

Adopt the same small helper to create the controller (with required=false in this test) to keep tests consistent and terse.

Also applies to: 278-279

router-tests/websocket_test.go (1)

190-192: Reduce repetition: extract a tiny helper for AccessController setup

This exact pattern repeats throughout the file; a helper will shrink noise and ease future API tweaks.

Apply locally where used:

-		accessController, err := core.NewAccessController(authenticators, false, core.IntrospectionAuthModeFull, "")
-		require.NoError(t, err)
+		accessController := mustAccessController(t, authenticators, false)

Add once in this test file:

func mustAccessController(t *testing.T, authenticators []authentication.Authenticator, required bool) *core.AccessController {
	t.Helper()
	ac, err := core.NewAccessController(authenticators, required, core.IntrospectionAuthModeFull, "")
	require.NoError(t, err)
	return ac
}

router-tests/telemetry/telemetry_test.go (1)

9031-9033: DRY up repeated controller construction across subtests

The NewAccessController + require.NoError pattern is repeated in several subtests. Consider a tiny helper to reduce noise and make future signature changes easier.

Apply this local replacement in one spot as an example:

-	authenticators, authServer := integration.ConfigureAuth(t)
-	accessController, err := core.NewAccessController(authenticators, false, core.IntrospectionAuthModeFull, "")
-	require.NoError(t, err)
+	authenticators, authServer := integration.ConfigureAuth(t)
+	accessController := mustNewAccessController(t, authenticators)
...
-				RouterOptions: []core.Option{
-					core.WithAccessController(accessController),
-				},
+				RouterOptions: []core.Option{
+					core.WithAccessController(accessController),
+				},

Add this helper in the same file (or a shared test util):

// at file scope
// import "github.com/wundergraph/cosmo/router/pkg/authentication"

func mustNewAccessController(t *testing.T, authenticators []authentication.Authenticator) *core.AccessController {
	t.Helper()
	ac, err := core.NewAccessController(authenticators, false, core.IntrospectionAuthModeFull, "")
	require.NoError(t, err)
	return ac
}

Also applies to: 9047-9048

router/pkg/config/config.go (1)

986-990: Add type-safety and env-only validation for AuthenticationMode.
Consider using a typed alias matching core.IntrospectionAuthMode and validating env-only runs (when no YAML is loaded) to avoid invalid values slipping through.

Apply:

 type IntrospectionConfiguration struct {
   Enabled            bool   `yaml:"enabled" envDefault:"true" env:"INTROSPECTION_ENABLED"`
-  AuthenticationMode string `yaml:"authentication_mode" envDefault:"full" env:"INTROSPECTION_AUTHENTICATION_MODE"`
+  AuthenticationMode string `yaml:"authentication_mode" envDefault:"full" env:"INTROSPECTION_AUTHENTICATION_MODE"`
   Token              string `yaml:"token" env:"INTROSPECTION_TOKEN"`
 }

And after env.Parse in LoadConfig, validate:

 if err := env.Parse(&cfg.Config); err != nil {
   return nil, err
 }
+// Validate env-only introspection mode (schema validation won’t run without YAML)
+switch cfg.Config.IntrospectionConfig.AuthenticationMode {
+case "full", "token", "skip":
+default:
+  return nil, fmt.Errorf("invalid INTROSPECTION_AUTHENTICATION_MODE: %q (allowed: full|token|skip)", cfg.Config.IntrospectionConfig.AuthenticationMode)
+}

router/core/graphql_prehandler.go (1)

358-389: Expose an attribute when auth is bypassed for observability.
Helps tracing/metrics dashboards distinguish bypassed flows.

Apply:

-            if !skipAuth {
+            if !skipAuth {
               _, authenticateSpan := h.tracer.Start(r.Context(), "Authenticate",
                 trace.WithSpanKind(trace.SpanKindServer),
                 trace.WithAttributes(requestContext.telemetry.traceAttrs...),
               )
               …
             }
+            if skipAuth {
+              routerSpan.SetAttributes(attribute.Bool("wg.auth.bypassed", true))
+            }

router/pkg/config/config.schema.json (1)

1369-1400: Tighten schema docs and hide secret in tooling.
Add descriptions and mark token writeOnly; warn against production “skip”.

   "introspection": {
     "type": "object",
-    "description": "",
+    "description": "GraphQL introspection settings. For production, prefer 'full' or 'token'. Using 'skip' is not recommended.",
     "additionalProperties": false,
     "properties": {
       "enabled": {
         "type": "boolean",
-        "description": "",
+        "description": "Enable GraphQL introspection.",
         "default": true
       },
       "authentication_mode": {
         "type": "string",
         "enum": ["full", "token", "skip"],
-        "description": "Authentication mode for introspection queries. 'full' requires normal authentication, 'token' requires a specific introspection token, 'skip' bypasses authentication entirely.",
+        "description": "How to authenticate introspection queries. 'full' = require normal auth; 'token' = require a dedicated introspection token; 'skip' = no auth (not recommended for production).",
         "default": "full"
       },
       "token": {
         "type": "string",
-        "description": "Token to authenticate introspection requests. Required when introspection.authentication_mode is set to 'token', ignored otherwise.",
+        "description": "Static secret for introspection requests when authentication_mode = 'token'.",
+        "writeOnly": true,
         "minLength": 32
       }
     },

router/core/access_controller.go (1)

35-39: Name nit: introspectionAuthSkipToken reads oddly in “token” mode.
Consider renaming to introspectionAuthToken for clarity.

router-tests/authentication_test.go (4)

2818-2823: Test name/comment mismatch with behavior (ModeSkip ignores token)

You configure a token but run in IntrospectionAuthModeSkip where the token is intentionally ignored. Rename/comment to reflect that.

Apply:

-    t.Run("introspection query with auth skip and introspection token", func(t *testing.T) {
+    t.Run("introspection query with auth skip (token ignored)", func(t *testing.T) {
@@
-        // since we are skipping auth for introspection and have not configured
-        // a dedicated token for introspection queries, this query must succeed
+        // ModeSkip bypasses auth for introspection; any configured token is ignored.

---

2850-2857: Test name mismatch: not “auth skip”, this is token mode

This subtest uses IntrospectionAuthModeToken; rename for clarity.

-    t.Run("introspection query with auth skip and invalid introspection token", func(t *testing.T) {
+    t.Run("introspection query with token mode and invalid introspection token", func(t *testing.T) {

---

2701-2939: Add positive coverage for token mode and clarify header format

You cover ModeToken (negative) but not the happy path. Also, header format for the introspection token (raw vs "Bearer ") isn’t asserted.

• Add a passing case for ModeToken:

+   t.Run("introspection query with token mode and valid introspection token", func(t *testing.T) {
+       t.Parallel()
+       authenticators, _ := ConfigureAuth(t)
+       token := "wg_test_introspection_token" // test-only token; gitleaks:allow
+       accessController, err := core.NewAccessController(authenticators, true, core.IntrospectionAuthModeToken, token)
+       require.NoError(t, err)
+       testenv.Run(t, &testenv.Config{
+           RouterOptions: []core.Option{core.WithAccessController(accessController)},
+       }, func(t *testing.T, xEnv *testenv.Environment) {
+           header := http.Header{"Authorization": []string{token}}
+           res, err := xEnv.MakeRequest(http.MethodPost, "/graphql", header, strings.NewReader(simpleIntrospectionQuery))
+           require.NoError(t, err)
+           defer res.Body.Close()
+           require.Equal(t, http.StatusOK, res.StatusCode)
+           require.Equal(t, "", res.Header.Get(xAuthenticatedByHeader))
+           data, _ := io.ReadAll(res.Body)
+           require.Equal(t, simpleIntrospectionExpectedData, string(data))
+       })
+   })

• If ModeToken accepts a Bearer prefix as well, mirror the above with:

header := http.Header{"Authorization": []string{"Bearer " + token}}

Please confirm expected header semantics; if only raw is supported, we should document that invariant in the test name/comments.

---

2701-2939: Minor duplication: helper for AccessController setup

Consider a tiny local helper to cut the repeated NewAccessController boilerplate across subtests and reduce mistakes.

Example:

newAC := func(t *testing.T, authRequired bool, mode core.IntrospectionAuthMode, token string, auths ...authentication.Authenticator) *core.AccessController {
  ac, err := core.NewAccessController(auths, authRequired, mode, token)
  require.NoError(t, err)
  return ac
}

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: SkArchon
PR: wundergraph/cosmo#2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.

Learnt from: endigma
PR: wundergraph/cosmo#2181
File: router/core/operation_processor.go:0-0
Timestamp: 2025-09-02T12:52:27.677Z
Learning: Hash validation for persisted queries with query bodies is performed in router/core/graphql_prehandler.go in the handleOperation function, not in the APQ processing logic in operation_processor.go. This validates that extensions.persistedQuery.sha256Hash matches the query body before any APQ operations occur.

Learnt from: endigma
PR: wundergraph/cosmo#2181
File: router/core/operation_processor.go:0-0
Timestamp: 2025-09-02T12:52:27.677Z
Learning: Hash validation for persisted queries with query bodies is performed in router/core/graphql_prehandler.go in the handleOperation function at lines 571-578, not in the APQ processing logic in operation_processor.go. This validates that extensions.persistedQuery.sha256Hash matches the computed query hash before any APQ operations occur. There's also a test case that verifies this behavior.

router/pkg/config/config_test.go (3)

1297-1371: Add happy-path for authentication_mode: skip

Include a positive test to lock in acceptance of skip mode.

Apply this diff near the end of TestValidateIntrospectionAuthConfig:

@@
 	t.Run("token too short", func(t *testing.T) {
 		t.Parallel()
@@
 		require.ErrorContains(t, err, "at '/introspection/token': minLength: got 15, want 32")
 	})
+
+	t.Run("valid config with skip mode", func(t *testing.T) {
+		t.Parallel()
+
+		f := createTempFileFromFixture(t, `
+version: "1"
+
+introspection:
+  enabled: true
+  authentication_mode: skip
+`)
+		_, err := LoadConfig([]string{f})
+		require.NoError(t, err)
+	})

---

1297-1371: Add boundary test for token length = 32

Prevents regressions on minLength enforcement.

Apply this diff:

@@
 	t.Run("valid config with token", func(t *testing.T) {
 		t.Parallel()
@@
 		require.NoError(t, err)
 	})
 
+	t.Run("valid token with exact 32 chars", func(t *testing.T) {
+		t.Parallel()
+		// 32 'a' characters
+		f := createTempFileFromFixture(t, `
+version: "1"
+
+introspection:
+  enabled: true
+  authentication_mode: token
+  token: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+`)
+		_, err := LoadConfig([]string{f})
+		require.NoError(t, err)
+	})

---

1297-1371: Optional: DRY up YAML fixture generation

A tiny helper reduces duplication and makes future cases cheaper to add.

I can push a helper like:

func introspectionYAML(enabled bool, mode, token string) string {
	tok := ""
	if token != "" {
		tok = fmt.Sprintf("\n  token: %s", token)
	}
	return fmt.Sprintf(`
version: "1"

introspection:
  enabled: %t
  authentication_mode: %s%s
`, enabled, mode, tok)
}

Then subtests can pass createTempFileFromFixture(t, introspectionYAML(true, "token", strings.Repeat("a", 32))).

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: SkArchon
PR: wundergraph/cosmo#2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.

router-tests/authentication_test.go (1)

42-49: Optional: factor a tiny helper to reduce repetition

AccessControllerOptions blocks repeat across many tests. Consider a small helper (in test utils) to build the controller to DRY this up.

Also applies to: 71-79, 103-111, 138-145, 164-171, 197-204, 232-239, 266-273, 300-308, 340-347, 376-384, 410-417, 441-448, 511-518, 631-639, 665-672, 700-707, 735-742, 770-777, 831-838, 905-911, 933-940, 965-972, 1028-1035, 1095-1101, 1146-1152, 1193-1199, 1241-1247, 1335-1341, 1641-1647, 1731-1737, 1761-1767, 1791-1797, 1826-1832, 1860-1866, 1949-1955, 1986-1992, 2023-2029, 2060-2065, 2097-2103, 2134-2139, 2175-2181, 2212-2217, 2249-2254, 2290-2295, 2331-2336, 2368-2373, 2405-2410, 2444-2450, 2486-2492, 2551-2556, 2599-2605, 2651-2656, 2695-2701, 2756-2761, 2801-2806, 2856-2862, 2901-2906, 2954-2961, 3002-3008

router/core/supervisor_instance.go (2)

73-80: Gate warnings when auth is configured; avoid misleading logs

If there are no authenticators, these warnings can confuse (no auth is enforced at all). Guard them by checking authenticators present.

Apply:

- if cfg.Authentication.IgnoreIntrospection && cfg.IntrospectionConfig.Secret == "" {
+ if len(authenticators) > 0 && cfg.Authentication.IgnoreIntrospection && cfg.IntrospectionConfig.Secret == "" {
   params.Logger.Warn("introspection operations are not authenticated. " +
     "Consider setting /introspection/secret or set /authentication/ignore_introspection to false. Do not use this in production.")
 }
- if !cfg.Authentication.IgnoreIntrospection && cfg.IntrospectionConfig.Secret != "" {
+ if len(authenticators) > 0 && !cfg.Authentication.IgnoreIntrospection && cfg.IntrospectionConfig.Secret != "" {
   params.Logger.Warn("/introspection/secret is ignored because /authentication/ignore_introspection is false.")
 }

---

171-179: Prefer the new nested flag as source of truth

Pass IntrospectionConfig.Enabled to WithIntrospection to avoid ambiguity with the deprecated boolean.

Apply:

-        WithIntrospection(config.IntrospectionEnabled, config.IntrospectionConfig),
+        WithIntrospection(config.IntrospectionConfig.Enabled, config.IntrospectionConfig),

router/core/access_controller.go (2)

35-37: Fix stale constructor comment (no invalid “auth mode”).

The comment mentions an introspection auth mode that no longer exists and suggests non-nil errors that aren’t produced.

Apply:

-// NewAccessController creates a new AccessController.
-// It returns an error if the introspection auth mode is invalid.
+// NewAccessController creates a new AccessController.
+// Note: currently returns a nil error; the error return is reserved for future configuration validation.

---

71-71: Rename unused param to underscore.

Communicates intent and appeases linters without API break.

Apply:

-func (a *AccessController) IntrospectionAccess(r *http.Request, body []byte) bool {
+func (a *AccessController) IntrospectionAccess(r *http.Request, _ []byte) bool {

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: endigma
PR: wundergraph/cosmo#2181
File: router/core/operation_processor.go:0-0
Timestamp: 2025-09-02T12:52:27.677Z
Learning: Hash validation for persisted queries with query bodies is performed in router/core/graphql_prehandler.go in the handleOperation function, not in the APQ processing logic in operation_processor.go. This validates that extensions.persistedQuery.sha256Hash matches the query body before any APQ operations occur.

Learnt from: endigma
PR: wundergraph/cosmo#2181
File: router/core/operation_processor.go:0-0
Timestamp: 2025-09-02T12:52:27.677Z
Learning: Hash validation for persisted queries with query bodies is performed in router/core/graphql_prehandler.go in the handleOperation function at lines 571-578, not in the APQ processing logic in operation_processor.go. This validates that extensions.persistedQuery.sha256Hash matches the computed query hash before any APQ operations occur. There's also a test case that verifies this behavior.

Learnt from: dkorittki
PR: wundergraph/cosmo#2192
File: router/core/access_controller.go:3-10
Timestamp: 2025-09-10T12:33:59.146Z
Learning: For introspection token authentication in the router, the Authorization header should contain the exact static secret as configured, without Bearer prefix or whitespace normalization. Any deviation should be treated as invalid authentication.

Learnt from: SkArchon
PR: wundergraph/cosmo#2224
File: router/pkg/authentication/keyfunc/keyfunc_test.go:167-178
Timestamp: 2025-09-22T11:13:39.415Z
Learning: When reviewing forked code from external libraries, gitleaks warnings for hardcoded test tokens/credentials should typically be ignored as they are legitimate test fixtures from the upstream source, not real sensitive data.

Learnt from: SkArchon
PR: wundergraph/cosmo#2224
File: router/pkg/authentication/jwks_token_decoder.go:176-199
Timestamp: 2025-09-17T18:13:31.021Z
Learning: In router/pkg/authentication/jwks_token_decoder.go, when AllowEmptyAlgorithm is true, the allowedAlgorithms slice is populated by getSupportedAlgorithms() from validation_store.go, ensuring it's never empty and preventing unintended token rejections. The validation store maintains its own internal supported algorithms map that gets returned regardless of input parameters.

Learnt from: SkArchon
PR: wundergraph/cosmo#2224
File: router/pkg/authentication/jwks_token_decoder.go:176-199
Timestamp: 2025-09-17T18:13:31.021Z
Learning: In router/pkg/authentication/jwks_token_decoder.go, when AllowEmptyAlgorithm is true, the allowedAlgorithms slice is populated by getSupportedAlgorithms() from validation_store.go, ensuring it's never empty and preventing unintended token rejections.

router/pkg/config/fixtures/full.yaml (1)

23-25: Introspection block looks good; add an inline hint to reduce confusion

Secret is only honored when authentication.ignore_introspection is true. Add a short comment to make that clear in the fixture.

 introspection:
   enabled: true
-  secret: 'AN_EXAMPLE_PLACEHOLDER_SECRET_ONLY'
+  secret: 'AN_EXAMPLE_PLACEHOLDER_SECRET_ONLY'
+  # Note: secret is only used when authentication.ignore_introspection is true

router/pkg/config/config.schema.json (2)

1369-1385: Fill missing descriptions for introspection and fix grammar

Descriptions are empty; add concise text and improve the secret note.

-    "introspection": {
-      "type": "object",
-      "description": "",
+    "introspection": {
+      "type": "object",
+      "description": "Configuration for GraphQL introspection.",
       "additionalProperties": false,
       "properties": {
         "enabled": {
           "type": "boolean",
-          "description": "",
+          "description": "Enable GraphQL introspection.",
           "default": true
         },
         "secret": {
           "type": "string",
-          "description": "A dedicated secret to authenticate introspection requests independently from the regular authentication. Need to passed by introspection queries via the 'Authorization' header. Works only when authentication.ignore_introspection is true.",
+          "description": "Optional secret to authenticate introspection requests independently from regular auth. Must be passed via the 'Authorization' header. Effective only when authentication.ignore_introspection is true.",
           "minLength": 32
         }
       }
     },

Optional (nice-to-have): enforce interaction via an if/then at the root so that when introspection.secret is present, authentication.ignore_introspection must be true.

---

1851-1855: Fix grammar in ignore_introspection description

Minor copy tweak.

-          "description": "If the value is true, introspection requests not need to be authenticated. The default value is false.",
+          "description": "If true, introspection requests do not need to be authenticated. The default value is false.",

router/core/supervisor_instance.go (2)

73-77: Update warning logic and messaging to the new config path

Use the new flag and reference /introspection/skip_authentication in the message.

Apply this diff:

-	if cfg.Authentication.IgnoreIntrospection && cfg.IntrospectionConfig.Secret == "" {
-		params.Logger.Warn("introspection operations are not authenticated. " +
-			"Consider setting /introspection/secret or set /authentication/ignore_introspection to false. Do not use this in production.")
-	}
+	if cfg.IntrospectionConfig.SkipAuthentication && cfg.IntrospectionConfig.Secret == "" {
+		params.Logger.Warn("introspection operations are not authenticated. " +
+			"Consider setting /introspection/secret or set /introspection/skip_authentication to false. Do not use this in production.")
+	}

---

78-80: Align second warning with new config path

Reference /introspection/skip_authentication instead of the deprecated field.

Apply this diff:

-	if !cfg.Authentication.IgnoreIntrospection && cfg.IntrospectionConfig.Secret != "" {
-		params.Logger.Warn("/introspection/secret is ignored because /authentication/ignore_introspection is false.")
-	}
+	if !cfg.IntrospectionConfig.SkipAuthentication && cfg.IntrospectionConfig.Secret != "" {
+		params.Logger.Warn("/introspection/secret is ignored because /introspection/skip_authentication is false.")
+	}

router-tests/authentication_test.go (1)

45-52: Reduce duplication: helper for controller setup in tests

Consider a small test helper to build the controller to cut repetition and prevent future drift:

func newAccessControllerForTests(t *testing.T, auths []authentication.Authenticator, required bool, skipIntrospection bool, secret string) *core.AccessController {
	ac, err := core.NewAccessController(core.AccessControllerOptions{
		Authenticators:           auths,
		AuthenticationRequired:   required,
		SkipIntrospectionQueries: skipIntrospection,
		IntrospectionSkipSecret:  secret,
	})
	require.NoError(t, err)
	return ac
}

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: SkArchon
PR: wundergraph/cosmo#2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.

Learnt from: SkArchon
PR: wundergraph/cosmo#2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.

Learnt from: SkArchon
PR: wundergraph/cosmo#2224
File: router/pkg/authentication/keyfunc/keyfunc_test.go:167-178
Timestamp: 2025-09-22T11:13:39.415Z
Learning: When reviewing forked code from external libraries, gitleaks warnings for hardcoded test tokens/credentials should typically be ignored as they are legitimate test fixtures from the upstream source, not real sensitive data.

Learnt from: SkArchon
PR: wundergraph/cosmo#2224
File: router/pkg/authentication/keyfunc/keyfunc_test.go:142-154
Timestamp: 2025-09-22T11:13:45.607Z
Learning: When reviewing forked code, especially test files with test fixtures like JWT tokens, avoid suggesting modifications to maintain alignment with upstream and preserve the original author's structure. Test fixtures that are clearly marked as such (not real secrets) should generally be left unchanged in forked implementations.

Learnt from: SkArchon
PR: wundergraph/cosmo#2224
File: router/pkg/authentication/jwks_token_decoder.go:176-199
Timestamp: 2025-09-17T18:13:31.021Z
Learning: In router/pkg/authentication/jwks_token_decoder.go, when AllowEmptyAlgorithm is true, the allowedAlgorithms slice is populated by getSupportedAlgorithms() from validation_store.go, ensuring it's never empty and preventing unintended token rejections. The validation store maintains its own internal supported algorithms map that gets returned regardless of input parameters.

Learnt from: SkArchon
PR: wundergraph/cosmo#2224
File: router/pkg/authentication/jwks_token_decoder.go:176-199
Timestamp: 2025-09-17T18:13:31.021Z
Learning: In router/pkg/authentication/jwks_token_decoder.go, when AllowEmptyAlgorithm is true, the allowedAlgorithms slice is populated by getSupportedAlgorithms() from validation_store.go, ensuring it's never empty and preventing unintended token rejections.

router-tests/authentication_test.go (1)

44-56: Reduce test duplication: extract AccessController builder helper

The options-based AccessController construction repeats throughout the file. Consider a small test helper to DRY it up.

Example helper (place in this package’s test utils):

func newAccessControllerForTests(t *testing.T, authenticators []authentication.Authenticator, requireAuth, skipIntrospection bool, secret string) *core.AccessController {
	t.Helper()
	ac, err := core.NewAccessController(core.AccessControllerOptions{
		Authenticators:           authenticators,
		AuthenticationRequired:   requireAuth,
		SkipIntrospectionQueries: skipIntrospection,
		IntrospectionSkipSecret:  secret,
	})
	require.NoError(t, err)
	return ac
}

Then replace repeated blocks with:

accessController := newAccessControllerForTests(t, authenticators, true, false, "")

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: SkArchon
PR: wundergraph/cosmo#2224
File: router/pkg/authentication/keyfunc/keyfunc_test.go:167-178
Timestamp: 2025-09-22T11:13:39.415Z
Learning: When reviewing forked code from external libraries, gitleaks warnings for hardcoded test tokens/credentials should typically be ignored as they are legitimate test fixtures from the upstream source, not real sensitive data.

Learnt from: SkArchon
PR: wundergraph/cosmo#2224
File: router/pkg/authentication/jwks_token_decoder.go:176-199
Timestamp: 2025-09-17T18:13:31.021Z
Learning: In router/pkg/authentication/jwks_token_decoder.go, when AllowEmptyAlgorithm is true, the allowedAlgorithms slice is populated by getSupportedAlgorithms() from validation_store.go, ensuring it's never empty and preventing unintended token rejections. The validation store maintains its own internal supported algorithms map that gets returned regardless of input parameters.

Learnt from: SkArchon
PR: wundergraph/cosmo#2224
File: router/pkg/authentication/jwks_token_decoder.go:176-199
Timestamp: 2025-09-17T18:13:31.021Z
Learning: In router/pkg/authentication/jwks_token_decoder.go, when AllowEmptyAlgorithm is true, the allowedAlgorithms slice is populated by getSupportedAlgorithms() from validation_store.go, ensuring it's never empty and preventing unintended token rejections.

Learnt from: SkArchon
PR: wundergraph/cosmo#2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.

router/pkg/config/config.go (1)

506-508: Unify introspection auth config; avoid split/precedence pitfalls

You’re introducing both Authentication.IgnoreIntrospection and a new Introspection section. This can create ambiguous precedence (e.g., ignore_introspection=true vs. requiring a secret/token under introspection). Prefer a single source of truth under Introspection to steer auth behavior and mark ignore_introspection as deprecated, or wire clear precedence in post-processing. Also, the PR narrative uses “token”, while the struct uses “secret” — that’s a doc/schema drift risk.

Recommended:

• Consolidate knobs under Introspection (e.g., authentication_mode: full|token|skip and token).
• Keep current fields for compatibility but document precedence and deprecations.

If you keep both for now, please confirm schema/docs reflect the final shape and precedence.

Minimal schema-alignment patch (add token alias and optional mode without breaking existing secret/env):

 type IntrospectionConfiguration struct {
-    Enabled bool   `yaml:"enabled" envDefault:"true" env:"INTROSPECTION_ENABLED"`
-    Secret  string `yaml:"secret" env:"INTROSPECTION_SECRET"`
+    Enabled            bool   `yaml:"enabled" envDefault:"true" env:"INTROSPECTION_ENABLED"`
+    // Token preferred in docs; keep Secret for back-compat with env while we transition.
+    Token              string `yaml:"token,omitempty" env:"INTROSPECTION_TOKEN"`
+    Secret             string `yaml:"secret,omitempty" env:"INTROSPECTION_SECRET"` // deprecated
+    AuthenticationMode string `yaml:"authentication_mode,omitempty" env:"INTROSPECTION_AUTHENTICATION_MODE"` // allowed: full|token|skip
 }

Optionally mark the new Authentication.IgnoreIntrospection as deprecated in favor of introspection.authentication_mode=skip, or move it under Introspection to avoid scattering related knobs.

Also applies to: 997-1000, 1028-1029

router-tests/websocket_test.go (1)

139-146: Adoption of AccessControllerOptions is correct; reduce duplication with a tiny helper

The per-test construction/injection of AccessController is repetitive but otherwise solid. Extract a helper to cut noise and make intent clearer.

Example helper for this file:

func newAccessControllerForTests(t *testing.T, auths []authentication.Authenticator, required bool) *core.AccessController {
  t.Helper()
  ac, err := core.NewAccessController(core.AccessControllerOptions{
    Authenticators:         auths,
    AuthenticationRequired: required,
    // Defaults for introspection behavior; override per test if needed.
  })
  require.NoError(t, err)
  return ac
}

Then replace blocks like:

accessController, err := core.NewAccessController(core.AccessControllerOptions{ ... })
require.NoError(t, err)
...
core.WithAccessController(accessController),

with:

core.WithAccessController(newAccessControllerForTests(t, authenticators, false)),

This trims several hundred repeated lines without changing behavior. Based on learnings.

Also applies to: 149-149, 196-203, 206-206, 252-259, 264-264, 318-325, 330-330, 384-390, 399-399, 453-459, 466-466, 509-515, 522-522, 916-923, 945-945

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: dkorittki
PR: wundergraph/cosmo#2192
File: router/core/supervisor_instance.go:59-71
Timestamp: 2025-09-23T16:27:54.358Z
Learning: In the router codebase, AccessController creation follows a nil pattern where accessController == nil signals that no authentication should be performed. Other parts of the code (like the prehandler) check for accessController == nil to determine whether to skip authentication entirely. Always creating an AccessController would break this pattern and cause authentication to be attempted even when no authenticators are configured.

Learnt from: SkArchon
PR: wundergraph/cosmo#2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: dkorittki
PR: wundergraph/cosmo#2192
File: router/pkg/config/config.go:1028-1029
Timestamp: 2025-09-29T10:28:07.088Z
Learning: The deprecation strategy for IntrospectionEnabled field in router/pkg/config/config.go is to first mark it as deprecated, then migrate all call sites to use IntrospectionConfig.Enabled, and finally remove the deprecated field. The envDefault tag should be kept during the deprecation period for backward compatibility.

Learnt from: dkorittki
PR: wundergraph/cosmo#2192
File: router/core/supervisor_instance.go:59-71
Timestamp: 2025-09-23T16:27:54.358Z
Learning: In the router codebase, AccessController creation follows a nil pattern where accessController == nil signals that no authentication should be performed. Other parts of the code (like the prehandler) check for accessController == nil to determine whether to skip authentication entirely. Always creating an AccessController would break this pattern and cause authentication to be attempted even when no authenticators are configured.

router/pkg/config/config.schema.json (1)

1860-1863: Fix grammar in ignore_introspection description.

The sentence should read “introspection requests do not need to be authenticated.” Please adjust the wording.

-      "description": "If the value is true, introspection requests not need to be authenticated. The default value is false.",
+      "description": "If the value is true, introspection requests do not need to be authenticated. The default value is false.",

router-tests/telemetry/telemetry_test.go (6)

9106-9112: Same refactor applies here (omit zero-values / optional helper)

Also applies to: 9125-9125

---

9161-9168: Same refactor applies here (omit zero-values / optional helper)

Also applies to: 9180-9180

---

9243-9249: Same refactor applies here (omit zero-values / optional helper)

Also applies to: 9272-9272

---

9326-9332: Same refactor applies here (omit zero-values / optional helper)

Also applies to: 9347-9347

---

9392-9398: Same refactor applies here (omit zero-values / optional helper)

Also applies to: 9411-9411

---

9601-9606: Same refactor applies here (omit zero-values / optional helper)

Also applies to: 9625-9625

router/core/graphql_prehandler.go (1)

646-675: Consider simplifying the nested conditionals.

The validation logic is correct but could be more readable with early returns or a helper function.

Optional refactor for improved readability:

 	if h.accessController != nil {
-		// Based on the authentication result, the introspection config,
-		// and wether this is an introspection query,
-		// we decide here if we need to abort the request or not.
 		isIntrospection, err := operationKit.isIntrospectionQuery()
 		if err != nil {
 			requestContext.logger.Error("failed to check if operation is introspection, treat it like non-introspection operation", zap.Error(err))
 			isIntrospection = false
 		}
 
-		// non-introspection queries are only allowed when authenticated via normal authentication
-		if !isIntrospection && httpOperation.authenticationPass != authenticationPassNormal {
+		// Determine if authentication is sufficient for this operation type
+		authAllowed := false
+		if !isIntrospection {
+			// Non-introspection requires normal authentication
+			authAllowed = (httpOperation.authenticationPass == authenticationPassNormal)
+		} else {
+			// Introspection allows normal auth, secret auth, or skip
+			authAllowed = (httpOperation.authenticationPass == authenticationPassNormal ||
+				httpOperation.authenticationPass == authenticationPassIntrospectionSecret ||
+				httpOperation.authenticationPass == authenticationPassSkip)
+		}
+
+		if !authAllowed {
 			return &httpGraphqlError{
 				message:    "unauthorized",
 				statusCode: http.StatusUnauthorized,
 			}
 		}
-
-		// introspection queries are only allowed when authenticated normally or via dedicated token, or when auth skip is enabled
-		// note: httpOperation.authMethod is only set when authentication is successful and the config allows such authentication.
-		if isIntrospection &&
-			httpOperation.authenticationPass != authenticationPassNormal &&
-			httpOperation.authenticationPass != authenticationPassIntrospectionSecret &&
-			httpOperation.authenticationPass != authenticationPassSkip {
-			return &httpGraphqlError{
-				message:    "unauthorized",
-				statusCode: http.StatusUnauthorized,
-			}
-		}
 	}

router/pkg/config/config.schema.json (1)

1374-1388: Document the introspection block.

The new introspection object currently has an empty description. Please add a short explanation (e.g., that it configures GraphQL introspection and is not recommended for production) so the schema docs stay usable.

router-tests/telemetry/telemetry_test.go (1)

9047-9054: Good adoption of NewAccessController and WithAccessController; remove redundant zero‑values and consider a small helper

• The usage is correct. You can drop explicit zero‑value options to reduce noise (bool false, empty string).
• Given the repetition across this file, consider a tiny test helper to construct the controller once.

Suggested minimal change here:

-accessController, err := core.NewAccessController(core.AccessControllerOptions{
-    Authenticators:           authenticators,
-    AuthenticationRequired:   false,
-    SkipIntrospectionQueries: false,
-    IntrospectionSkipSecret:  "",
-})
+accessController, err := core.NewAccessController(core.AccessControllerOptions{
+    Authenticators: authenticators,
+})
 require.NoError(t, err)

Optionally, introduce a helper and use it at call sites:

// in this package (test file), for reuse
func newTestAccessController(t *testing.T, authenticators []authentication.Authenticator) *core.AccessController {
  ac, err := core.NewAccessController(core.AccessControllerOptions{
    Authenticators: authenticators,
    // AuthenticationRequired false by default; add fields here only if a test needs them.
  })
  require.NoError(t, err)
  return ac
}

Then:

-accessController, err := core.NewAccessController(core.AccessControllerOptions{ ... })
-require.NoError(t, err)
+accessController := newTestAccessController(t, authenticators)

Also applies to: 9068-9068

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: dkorittki
PR: wundergraph/cosmo#2192
File: router/pkg/config/config.go:1028-1029
Timestamp: 2025-09-29T10:28:07.122Z
Learning: The deprecation strategy for IntrospectionEnabled field in router/pkg/config/config.go is to first mark it as deprecated, then migrate all call sites to use IntrospectionConfig.Enabled, and finally remove the deprecated field. The envDefault tag should be kept during the deprecation period for backward compatibility.

Learnt from: dkorittki
PR: wundergraph/cosmo#2192
File: router/core/supervisor_instance.go:59-71
Timestamp: 2025-09-23T16:27:54.358Z
Learning: In the router codebase, AccessController creation follows a nil pattern where accessController == nil signals that no authentication should be performed. Other parts of the code (like the prehandler) check for accessController == nil to determine whether to skip authentication entirely. Always creating an AccessController would break this pattern and cause authentication to be attempted even when no authenticators are configured.